Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Release/8.0] Fix FP state restore on macOS exception forwarding #109579

Merged
merged 1 commit into from
Dec 3, 2024
Merged

[Release/8.0] Fix FP state restore on macOS exception forwarding #109579

merged 1 commit into from
Dec 3, 2024

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Nov 6, 2024 •
edited
Loading

Backport of #105003, #109458 and part of #99255

Customer Impact

[x] Customer reported
[ ] Found internally

Original issue: #109423
Attempt to load Java VM into .NET process on macOS x64 always crashes due to a problem in hardware exception forwarding from coreclr's exception handling port to Java's exception handling port (or any other registered one).

Regression

[x] Yes
[ ] No

Regression in .NET 8.0.8 due to a change to fix AVX512 support on macOS for debugging.

Testing

Testing using a repro provided by the customer and coreclr tests

Risk

Low, fixes a buffer overflow, a stack overflow and obviously incorrect floating point context restoration in case of hardware exception in external non-.NET code.

@janvorli janvorli added Servicing-consider Issue for next servicing release review area-ExceptionHandling-coreclr labels Nov 6, 2024
@janvorli janvorli added this to the 8.0.x milestone Nov 6, 2024
@janvorli janvorli requested a review from VSadov November 6, 2024 11:35
@janvorli janvorli self-assigned this Nov 6, 2024
jeffschwMSFT
jeffschwMSFT approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. we will take for consideration in 8.0.x

@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 7, 2024
@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.12 Nov 7, 2024
@jonpryor jonpryor mentioned this pull request Nov 25, 2024
Draft
@jonpryor
Copy link
Member

jonpryor commented Nov 26, 2024 •
edited
Loading

It looks like this fix is also needed on .NET 9 + Intel + macOS. From (build log):

  # jonp: LoadJvmLibrary(/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.25-9/x64/Contents/Home/lib/jli/libjli.dylib)=140707647285392
  # jonp: JNI_CreateJavaVM=4419599558; JNI_GetCreatedJavaVMs=4419599627
  # jonp: executing JNI_CreateJavaVM=1076dbcc6
  /var/folders/p1/44pzfl0j2m301zzfb0fv0p380000gn/T/MSBuildTemprunner/tmp87eb9e12ad0742e0ac307f233a414bd3.exec.cmd: line 2:  8473 Floating point exception: 8   "/Users/runner/hostedtoolcache/dotnet/dotnet" "/Users/runner/work/1/s/bin/Release-net9.0//jnimarshalmethod-gen.dll" "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/Hello-NativeAOTFromJNI.dll" -v -v --keeptemp -L "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/"
##[error]samples/Hello-NativeAOTFromJNI/Hello-NativeAOTFromJNI.targets(44,5): Error MSB3073: The command ""/Users/runner/hostedtoolcache/dotnet/dotnet" "/Users/runner/work/1/s/bin/Release-net9.0//jnimarshalmethod-gen.dll" "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/Hello-NativeAOTFromJNI.dll" -v -v --keeptemp -L "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/" " exited with code 136.

Note the Floating point exception: 8 in the output.

@janvorli
Copy link
Member Author

It looks like this fix is also needed on .NET 9 + Intel + macOS.

Right, I've created a backporting PR to 9 for that today.

@janvorli janvorli merged commit 7db9d7b into dotnet:release/8.0-staging Dec 3, 2024
112 of 117 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffschwMSFT jeffschwMSFT jeffschwMSFT approved these changes

@VSadov VSadov VSadov approved these changes

Assignees

@janvorli janvorli

Labels
area-ExceptionHandling-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Milestone
8.0.12
Development

Successfully merging this pull request may close these issues.
5 participants
@janvorli @jonpryor @VSadov @jeffschwMSFT @rbhanda